Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(cat-voices): parse missing document properties, add validation #1503

Merged
merged 56 commits into from
Jan 20, 2025
Merged

feat(cat-voices): parse missing document properties, add validation #1503

merged 56 commits into from
Jan 20, 2025

Conversation

dtscalac
Copy link
Contributor

@dtscalac dtscalac commented Jan 10, 2025 •
edited
Loading

Description

Parses json schema array and object property types and adds validation.

  • Since json schema allows nesting, each property can include child properties.
  • Updates UI widgets to build recursively UI for corresponding properties until the full document graph is traversed.
  • When parsing: resigned from hierarchical approach: segment -> section -> property, new implementation allows infinite nesting.
  • When building UI: the hierarchy of segment -> section -> property -> property -> (...) is supported, any properties which don't follow this hierarchy are ignored for the proposal builder (i.e. segment -> property -> section - would ignore the whole node since on 2nd level there is no section)

DocumentSchema

  • replaces the BaseDocumentDefinition, there is a hierarchy of sealed classes based on the elementary json schema types (string, integer, number, boolean, array, object). These bases classes are extended by specific definitions (schemas) like DocumentSingleLineTextEntrySchema, etc.

DocumentObjectSchema

  • includes children properties but new items cannot be added. Think of it like a group for a set of fields. A segment/section is based on this type of schema.

DocumentListSchema

  • includes children properties and new items can be added so that the list of children is between <minItems, maxItems> range

DocumentValueSchema and subclasses

  • represent a single string, number (double), integer, boolean fields, no children allowed.

DocumentChange

  • updated to allow adding/removing list items from properties that use the DocumentListSchema schema.

TODOs

  • next PRs will add more tests
  • add more validation rules (oneOf, const, enum)

Related Issue(s)

Closes #1466

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

@dtscalac dtscalac added draft Draft dart Pull requests that update Dart code labels Jan 10, 2025
@dtscalac dtscalac added this to the M4: Voting & Delegation milestone Jan 10, 2025
@dtscalac dtscalac self-assigned this Jan 10, 2025
@github-actions GitHub Actions
Copy link
Contributor

❌ Test Report | ${\color{lightgreen}Pass: 374/385}$ | ${\color{red}Fail: 11/385}$ |

@github-actions GitHub Actions
Copy link
Contributor

❌ Test Report | ${\color{lightgreen}Pass: 370/384}$ | ${\color{red}Fail: 14/384}$ |

@github-actions GitHub Actions
Copy link
Contributor

❌ Test Report | ${\color{lightgreen}Pass: 372/384}$ | ${\color{red}Fail: 12/384}$ |

@damian-molinski damian-molinski mentioned this pull request Jan 15, 2025
Merged
8 tasks
@dtscalac
feat: wip
465df5e
@dtscalac
Merge branch 'main' into feat/parse-missing-document-properties
36318d0 
# Conflicts:
#	catalyst_voices/apps/voices/lib/widgets/tiles/document_builder_section_tile.dart
#	catalyst_voices/packages/internal/catalyst_voices_models/lib/src/document/definitions/nested_questions_definition.dart
#	catalyst_voices/packages/internal/catalyst_voices_models/lib/src/document/definitions/single_line_https_url_entry_definition.dart
#	catalyst_voices/packages/internal/catalyst_voices_models/lib/src/document/definitions/yes_no_choice_definition.dart
#	catalyst_voices/packages/internal/catalyst_voices_models/lib/src/document/document_validator.dart
#	catalyst_voices/packages/internal/catalyst_voices_repositories/lib/src/dto/document/schema/document_schema_property_dto.dart
@github-actions GitHub Actions
Copy link
Contributor

❌ Test Report | ${\color{lightgreen}Pass: 361/369}$ | ${\color{red}Fail: 8/369}$ |

@dtscalac dtscalac marked this pull request as ready for review January 17, 2025 12:18
@dtscalac dtscalac added review me PR is ready for review and removed draft Draft labels Jan 17, 2025
@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 403/403}$ | ${\color{red}Fail: 0/403}$ |

@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 403/403}$ | ${\color{red}Fail: 0/403}$ |

@dtscalac
Merge branch 'main' into feat/parse-missing-document-properties
d40f7de 
# Conflicts:
#	catalyst_voices/apps/voices/lib/widgets/tiles/document_builder_section_tile.dart
@dtscalac dtscalac mentioned this pull request Jan 17, 2025
Merged
8 tasks
@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 403/403}$ | ${\color{red}Fail: 0/403}$ |

damian-molinski
damian-molinski reviewed Jan 17, 2025
View reviewed changes
Copy link
Contributor

@damian-molinski damian-molinski left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving some comments. Most styling.

Didn't finished reviewing but i'm loving what i saw so far. Great job.

Will continue on monday

catalyst_voices/apps/voices/lib/widgets/document_builder/agreement_confirmation_widget.dart Outdated Show resolved Hide resolved
catalyst_voices/apps/voices/lib/widgets/tiles/document_builder_section_tile.dart Show resolved Hide resolved
catalyst_voices/apps/voices/lib/widgets/tiles/document_builder_section_tile.dart Show resolved Hide resolved
...ices/packages/internal/catalyst_voices_models/lib/src/document/builder/document_builder.dart Outdated Show resolved Hide resolved
...ices/packages/internal/catalyst_voices_models/lib/src/document/builder/document_builder.dart Outdated Show resolved Hide resolved
catalyst_voices/packages/internal/catalyst_voices_models/lib/src/document/document.dart Outdated Show resolved Hide resolved
damian-molinski

This comment was marked as resolved.

Sign in to view

@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 403/403}$ | ${\color{red}Fail: 0/403}$ |

@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 403/403}$ | ${\color{red}Fail: 0/403}$ |

@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 403/403}$ | ${\color{red}Fail: 0/403}$ |

damian-molinski
damian-molinski approved these changes Jan 20, 2025
View reviewed changes
Copy link
Contributor

@damian-molinski damian-molinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

LynxLynxx
LynxLynxx approved these changes Jan 20, 2025
View reviewed changes
Copy link
Contributor

@LynxLynxx LynxLynxx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 405/405}$ | ${\color{red}Fail: 0/405}$ |

@dtscalac dtscalac merged commit cf01c54 into main Jan 20, 2025
40 of 42 checks passed
@dtscalac dtscalac deleted the feat/parse-missing-document-properties branch January 20, 2025 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LynxLynxx LynxLynxx LynxLynxx approved these changes

@damian-molinski damian-molinski damian-molinski approved these changes

Assignees

@dtscalac dtscalac

Labels
dart Pull requests that update Dart code review me PR is ready for review
Projects
Catalyst
Archived in project
Milestone
M4: Voting & Delegation
Development

Successfully merging this pull request may close these issues.

🛠️ [TASK] : Parse and add validation logic for document properties
3 participants
@dtscalac @damian-molinski @LynxLynxx